refactor(datesets): 重构swebp镜像操作逻辑,优化管理粒度#384
Conversation
1. 归一swe和swebp两个数据集的镜像操作,消除冗余代码。 2. 优化swebp临时容器清理逻辑,以Session粒度管理生命周期
There was a problem hiding this comment.
Code Review
This pull request introduces session-scoped container cleanup for SWE-bench Pro tasks to prevent concurrent tasks from interfering with each other's containers. It parameterizes the existing SWE-bench session labeling utilities to support custom label keys and integrates them into SWE-bench Pro evaluation and inference. Feedback on the newly added tests highlights a potential issue where unconditionally stubbing Unix-only modules (fcntl and resource) can break subsequent tests, and points out that several cleanup tests mock the wrong function (list_swebench_pro_container_ids instead of list_swebench_container_ids), rendering the mocks and assertions ineffective.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| def _setup_stubs(): | ||
| # Unix-only modules referenced transitively by the ais_bench package. | ||
| for name in ("fcntl", "resource"): | ||
| if name not in sys.modules: | ||
| sys.modules[name] = types.ModuleType(name) |
There was a problem hiding this comment.
在 Unix 系统上,如果该测试先于其他测试运行,fcntl 和 resource 可能尚未加载到 sys.modules 中。此时,_setup_stubs 会将它们替换为不具备任何实际功能的虚拟模块(dummy modules)。当后续的其他测试或业务代码尝试导入并使用这些模块的真实方法(例如 fcntl.flock)时,将会抛出 AttributeError,从而导致测试套件在 Unix/Linux 环境下运行失败。建议仅在导入失败时才进行 stub。
| def _setup_stubs(): | |
| # Unix-only modules referenced transitively by the ais_bench package. | |
| for name in ("fcntl", "resource"): | |
| if name not in sys.modules: | |
| sys.modules[name] = types.ModuleType(name) | |
| def _setup_stubs(): | |
| # Unix-only modules referenced transitively by the ais_bench package. | |
| for name in ("fcntl", "resource"): | |
| if name not in sys.modules: | |
| try: | |
| __import__(name) | |
| except ImportError: | |
| sys.modules[name] = types.ModuleType(name) |
| def test_cleanup_removes_only_session_tagged_ids(self): | ||
| with patch.object( | ||
| utils_module, "list_swebench_pro_container_ids", return_value={"session-a", "session-b"} | ||
| ), patch.object(utils_module.subprocess, "run") as mock_run: | ||
| self.utils.cleanup_swebench_pro_containers(session_id="session-1") |
There was a problem hiding this comment.
在 test_cleanup_removes_only_session_tagged_ids 测试中,您尝试通过 patch.object(utils_module, "list_swebench_pro_container_ids", ...) 来模拟容器 ID 的返回。然而,cleanup_swebench_pro_containers 实际上调用的是 _cleanup_session_containers(即 swebench/utils.py 中的 cleanup_swebench_containers),而该函数内部直接调用的是 list_swebench_container_ids,而不是 list_swebench_pro_container_ids。因此,您对 list_swebench_pro_container_ids 的 Mock 并不会生效。这会导致 cleanup_swebench_containers 内部调用未被 Mock 的 list_swebench_container_ids,进而调用被 Mock 的 subprocess.run。由于 subprocess.run 默认返回的 Mock 对象的 returncode 不为 0,list_swebench_container_ids 将返回空集,导致 cleanup_swebench_containers 提前返回,最终使得 mock_run.assert_called_once_with 断言失败。建议直接 Mock ais_bench.benchmark.tasks.swebench.utils.list_swebench_container_ids。
| def test_cleanup_removes_only_session_tagged_ids(self): | |
| with patch.object( | |
| utils_module, "list_swebench_pro_container_ids", return_value={"session-a", "session-b"} | |
| ), patch.object(utils_module.subprocess, "run") as mock_run: | |
| self.utils.cleanup_swebench_pro_containers(session_id="session-1") | |
| def test_cleanup_removes_only_session_tagged_ids(self): | |
| with patch( | |
| "ais_bench.benchmark.tasks.swebench.utils.list_swebench_container_ids", return_value={"session-a", "session-b"} | |
| ), patch.object(utils_module.subprocess, "run") as mock_run: | |
| self.utils.cleanup_swebench_pro_containers(session_id="session-1") |
| def test_cleanup_handles_docker_not_found(self): | ||
| with patch.object( | ||
| utils_module, "list_swebench_pro_container_ids", return_value={"c1"} | ||
| ), patch.object( | ||
| utils_module.subprocess, "run", side_effect=FileNotFoundError("docker not found") | ||
| ): | ||
| # Should not raise even when docker CLI is missing. | ||
| self.utils.cleanup_swebench_pro_containers(session_id="session-1") |
There was a problem hiding this comment.
与 test_cleanup_removes_only_session_tagged_ids 类似,由于 Mock 的是 list_swebench_pro_container_ids 而不是实际调用的 list_swebench_container_ids,该测试实际上并没有测试到 cleanup_swebench_pro_containers 在 docker rm 抛出 FileNotFoundError 时的异常处理逻辑(因为 targets 为空,函数直接提前返回了)。建议直接 Mock ais_bench.benchmark.tasks.swebench.utils.list_swebench_container_ids。
| def test_cleanup_handles_docker_not_found(self): | |
| with patch.object( | |
| utils_module, "list_swebench_pro_container_ids", return_value={"c1"} | |
| ), patch.object( | |
| utils_module.subprocess, "run", side_effect=FileNotFoundError("docker not found") | |
| ): | |
| # Should not raise even when docker CLI is missing. | |
| self.utils.cleanup_swebench_pro_containers(session_id="session-1") | |
| def test_cleanup_handles_docker_not_found(self): | |
| with patch( | |
| "ais_bench.benchmark.tasks.swebench.utils.list_swebench_container_ids", return_value={"c1"} | |
| ), patch.object( | |
| utils_module.subprocess, "run", side_effect=FileNotFoundError("docker not found") | |
| ): | |
| # Should not raise even when docker CLI is missing. | |
| self.utils.cleanup_swebench_pro_containers(session_id="session-1") |
| def test_cleanup_handles_timeout(self): | ||
| with patch.object( | ||
| utils_module, "list_swebench_pro_container_ids", return_value={"c1"} | ||
| ), patch.object( | ||
| utils_module.subprocess, "run", | ||
| side_effect=subprocess.TimeoutExpired(cmd="docker", timeout=30), | ||
| ): | ||
| # Should not raise even when docker rm times out. | ||
| self.utils.cleanup_swebench_pro_containers(session_id="session-1") |
There was a problem hiding this comment.
与前两个测试类似,由于 Mock 的是 list_swebench_pro_container_ids 而不是实际调用的 list_swebench_container_ids,该测试实际上并没有测试到 cleanup_swebench_pro_containers 在 docker rm 超时时的异常处理逻辑(因为 targets 为空,函数直接提前返回了)。建议直接 Mock ais_bench.benchmark.tasks.swebench.utils.list_swebench_container_ids。
| def test_cleanup_handles_timeout(self): | |
| with patch.object( | |
| utils_module, "list_swebench_pro_container_ids", return_value={"c1"} | |
| ), patch.object( | |
| utils_module.subprocess, "run", | |
| side_effect=subprocess.TimeoutExpired(cmd="docker", timeout=30), | |
| ): | |
| # Should not raise even when docker rm times out. | |
| self.utils.cleanup_swebench_pro_containers(session_id="session-1") | |
| def test_cleanup_handles_timeout(self): | |
| with patch( | |
| "ais_bench.benchmark.tasks.swebench.utils.list_swebench_container_ids", return_value={"c1"} | |
| ), patch.object( | |
| utils_module.subprocess, "run", | |
| side_effect=subprocess.TimeoutExpired(cmd="docker", timeout=30), | |
| ): | |
| # Should not raise even when docker rm times out. | |
| self.utils.cleanup_swebench_pro_containers(session_id="session-1") |
Thanks for your contribution; we appreciate it a lot. The following instructions will make your pull request healthier and help you get feedback more easily. If you do not understand some items, don't worry, just make the pull request and seek help from maintainers.
感谢您的贡献,我们非常重视。以下说明将使您的拉取请求更健康,更易于获得反馈。如果您不理解某些项目,请不要担心,只需提交拉取请求并从维护人员那里寻求帮助即可。
PR Type / PR类型
Related Issue | 关联 Issue
Fixes #380
🔍 Motivation / 变更动机
📝 Modification / 修改内容
📐 Associated Test Results / 关联测试结果
Please provide links to the related test results, such as CI pipelines, test reports, etc.
请提供相关测试结果的链接,例如 CI 管道、测试报告等。
Does the modification introduce changes that break the backward compatibility of the downstream repositories? If so, please describe how it breaks the compatibility and how the downstream projects should modify their code to keep compatibility with this PR.
是否引入了会破坏下游存储库向后兼容性的更改?如果是,请描述它如何破坏兼容性,以及下游项目应该如何修改其代码以保持与此 PR 的兼容性。
If the modification introduces performance degradation, please describe the impact of the performance degradation and the expected performance improvement.
如果引入了性能下降,请描述性能下降的影响和预期的性能改进。
🌟 Use cases (Optional) / 使用案例(可选)
If this PR introduces a new feature, it is better to list some use cases here and update the documentation.
如果此拉取请求引入了新功能,最好在此处列出一些用例并更新文档。
✅ Checklist / 检查列表
Before PR:
After PR:
👥 Collaboration Info / 协作信息
🌟 Useful CI Command / 实用的CI命令
/gemini review/gemini summary/gemini help/readthedocs build